Skip to content

Fix #407: [Model] RootedTreeArrangement#742

Merged
GiggleLiu merged 5 commits intomainfrom
issue-407
Mar 22, 2026
Merged

Fix #407: [Model] RootedTreeArrangement#742
GiggleLiu merged 5 commits intomainfrom
issue-407

Conversation

@GiggleLiu
Copy link
Copy Markdown
Contributor

Summary

Add the implementation plan for RootedTreeArrangement from issue #407 and use this PR to track the model work.

Fixes #407

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 96.56863% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.61%. Comparing base (63b53e3) to head (e249df3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/models/graph/rooted_tree_arrangement.rs 94.77% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #742      +/-   ##
==========================================
- Coverage   97.63%   97.61%   -0.02%     
==========================================
  Files         439      441       +2     
  Lines       54251    54455     +204     
==========================================
+ Hits        52968    53158     +190     
- Misses       1283     1297      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GiggleLiu
Copy link
Copy Markdown
Contributor Author

Implementation Summary

Changes

  • Added RootedTreeArrangement as a new graph satisfaction model with parent-array plus permutation encoding, brute-force solver registration, canonical example metadata, and focused unit tests.
  • Wired the model into the crate exports, CLI pred create direct/random flows, CLI help, and a regression test for pred create RootedTreeArrangement.
  • Added a paper problem-def entry plus bibliography entries for Gavril (1977) and Adolphson-Hu (1973).

Deviations from Plan

  • The canonical example used for example-db and the paper is a 4-vertex triangle-plus-tail instance instead of the issue's 5-vertex Example 2. This keeps the search space under the repo's example-db brute-force optimality threshold while preserving the same rooted-tree-arrangement structure.

Open Questions

  • None.

@GiggleLiu
Copy link
Copy Markdown
Contributor Author

Agentic Review Report

Structural Check

Structural Review: model RootedTreeArrangement

Structural Completeness

# Check Status
1 Model file exists PASS — src/models/graph/rooted_tree_arrangement.rs was added.
2 inventory::submit! present PASS — present at src/models/graph/rooted_tree_arrangement.rs:13.
3 Serialize / Deserialize derive on struct PASS — present at src/models/graph/rooted_tree_arrangement.rs:30.
4 Problem trait impl PASS — present at src/models/graph/rooted_tree_arrangement.rs:98.
5 SatisfactionProblem impl PASS — present at src/models/graph/rooted_tree_arrangement.rs:119.
6 Test link via #[cfg(test)] + #[path = ...] PASS — present at src/models/graph/rooted_tree_arrangement.rs:226.
7 Test file exists PASS — src/unit_tests/models/graph/rooted_tree_arrangement.rs exists.
8 Test file has >= 3 test functions PASS — 7 test_... functions found.
9 Registered in graph/mod.rs PASS — module and re-export added in src/models/graph/mod.rs.
10 Re-exported in models/mod.rs PASS — RootedTreeArrangement re-exported in src/models/mod.rs:30.
11 declare_variants! entry exists PASS — present at src/models/graph/rooted_tree_arrangement.rs:209.
12 CLI resolve/lookup support PASS — catalog-driven lookup works; pred show RootedTreeArrangement resolves successfully without manual alias-table edits.
13 CLI create support PASS — direct and random create branches exist in problemreductions-cli/src/commands/create.rs:2927 and problemreductions-cli/src/commands/create.rs:5391.
14 Canonical model example registered PASS — canonical_model_example_specs() exists in the model file and pred create --example RootedTreeArrangement succeeded.
15 Paper display-name entry PASS — present in docs/paper/reductions.typ:121.
16 Paper problem-def block PASS — present in docs/paper/reductions.typ:1755.
17 Deterministic whitelist FAIL — the PR also changes problemreductions-cli/src/cli.rs, problemreductions-cli/src/commands/create.rs, problemreductions-cli/tests/cli_tests.rs, and src/lib.rs, which are outside the model-PR whitelist.
18 Blacklisted autogenerated files absent PASS — none of the banned generated artifacts appear in the diff.

Build Status

  • make test: PASS
  • make clippy: PASS

Semantic Review

  • evaluate() correctness: OK — the implementation rejects invalid parent arrays, invalid permutations, non-ancestor-comparable mapped edges, and over-bound embeddings before returning true.
  • dims() correctness: OK — the encoding is 2n variables over [0, n), matching the stated parent-array plus permutation representation.
  • Size getter consistency: OK — num_vertices() / num_edges() exist and match the names used by metadata and exported schema.
  • Complexity metadata: ISSUE — declare_variants! advertises 2^num_vertices at src/models/graph/rooted_tree_arrangement.rs:209, but the only implemented exact solver path is brute force over dims() = vec![n; 2*n] at src/models/graph/rooted_tree_arrangement.rs:109, i.e. an n^(2n) search space. The catalog now understates the implemented complexity.

Issue Compliance

# Check Status
1 Problem name matches issue OK — RootedTreeArrangement.
2 Mathematical definition matches OK — edge comparability plus total tree-distance bound are implemented.
3 Problem type (opt/sat) matches OK — satisfaction problem with Metric = bool.
4 Type parameters match OK — graph-parameterized model with SimpleGraph default variant.
5 Configuration space matches OK — parent-array plus permutation encoding.
6 Feasibility check matches OK — tree validity, bijection validity, ancestor comparability, and bound are all enforced.
7 Objective / acceptance criterion matches OK — accepts iff total stretch is at most bound.
8 Complexity matches linked issue text OK — the linked issue currently asks for "2^num_vertices", though the metadata remains misleading in practice (see Semantic Review).

Summary

  • 17/18 structural checks passed.
  • 8/8 issue compliance checks passed.
  • FAIL / ISSUE items:
  • Deterministic whitelist failed because the PR includes CLI / crate-export wiring outside the model-PR whitelist.
  • Complexity metadata understates the implemented brute-force search space (2^n reported vs n^(2n) encoded).

Quality Check

Quality Review

Design Principles

  • DRY: OK — the model-specific logic is centralized in src/models/graph/rooted_tree_arrangement.rs, and the CLI changes follow the existing create-branch pattern used by neighboring graph models.
  • KISS: OK — parent-array + permutation is the minimal encoding that fits the repo’s Problem / BruteForce architecture.
  • HC/LC: OK — model logic, CLI wiring, tests, and paper text remain separated cleanly by module.

HCI (CLI changed)

  • Error messages: OK — missing --bound is reported with a specific, actionable message in problemreductions-cli/src/commands/create.rs:2931.
  • Discoverability: OK — pred list and pred show RootedTreeArrangement expose the new model correctly.
  • Consistency: OK — direct and random create flows mirror the existing OptimalLinearArrangement UX.
  • Least surprise: ISSUE — the shipped paper command block uses bare pred solve rooted-tree-arrangement.json (docs/paper/reductions.typ:1764), but the model has no ILP path, so the default solver fails on the canonical example.
  • Feedback: OK — create/evaluate output is clear and machine-readable.

Test Quality

  • Naive test detection: ISSUE
    • problemreductions-cli/tests/cli_tests.rs:3012 only checks JSON creation for pred create RootedTreeArrangement; there is no CLI regression for pred create --example RootedTreeArrangement plus solving the example. That gap allowed the broken canonical pred solve command to ship.

Issues

Critical (Must Fix)

  • None.

Important (Should Fix)

  • Canonical user path is broken: pred create --example RootedTreeArrangement works, but pred solve <example> fails with No reduction path from RootedTreeArrangement to ILP...; users must discover --solver brute-force themselves. The paper currently documents the failing command at docs/paper/reductions.typ:1765-1767.
  • Complexity metadata is inaccurate: pred list / pred show now advertise O(2^num_vertices) from src/models/graph/rooted_tree_arrangement.rs:209, even though the implementation’s encoded brute-force space is n^(2n) via src/models/graph/rooted_tree_arrangement.rs:109-111.

Minor (Nice to Have)

  • The deterministic model-PR whitelist fails because this PR also updates CLI/crate-export files. That may be intentional, but it reduces the signal of the whitelist gate.

Summary

  • The code structure is solid and the CLI create flows work, but the canonical solve command is wrong for the shipped example.
  • The new model’s registry complexity metadata is materially optimistic relative to the implementation.
  • CLI coverage is too narrow to catch the broken example solve path.

Agentic Feature Tests

Feature Test Report: problem-reductions

Date: 2026-03-22
Project type: CLI + Rust library
Features tested: RootedTreeArrangement
Profile: ephemeral
Use Case: Discover the new model from the catalog, materialize its canonical example, solve it from the documented command path, and verify the witness from the CLI alone.
Expected Outcome: pred list / pred show should surface the model, pred create --example RootedTreeArrangement should create a valid instance, and the documented pred solve rooted-tree-arrangement.json path should solve that example successfully.
Verdict: fail
Critical Issues: 0

Summary

Feature Discoverable Setup Works Expected Outcome Met Doc Quality
RootedTreeArrangement yes yes partial no paper solve command is incorrect

Per-Feature Details

RootedTreeArrangement

  • Role: downstream CLI user validating the new model from docs/examples.
  • Use Case: inspect the model, create its canonical example, solve it, and verify the published witness.
  • What they tried: pred list, pred show RootedTreeArrangement, pred create --example RootedTreeArrangement -o /tmp/.../example.json, pred solve /tmp/.../example.json, pred evaluate /tmp/.../example.json --config 0,0,1,2,0,1,2,3, pred solve /tmp/.../example.json --solver brute-force, direct pred create RootedTreeArrangement ..., and random pred create RootedTreeArrangement --random ....
  • Discoverability: Good. The model appears in pred list, and pred show prints fields and complexity metadata.
  • Setup: Worked. Building/installing pred from the PR worktree succeeded.
  • Functionality: Partial. pred create --example, direct create, random create, pred evaluate, and pred solve --solver brute-force all worked. Bare pred solve on the canonical example failed.
  • Expected vs Actual Outcome: Expected the paper’s canonical pred solve rooted-tree-arrangement.json command to work as written. Actual behavior requires --solver brute-force because pred path RootedTreeArrangement ILP reports no reduction path.
  • Blocked steps: None.
  • Friction points: The default-solver failure is surprising because the shipped paper command omits the required solver flag.
  • Doc suggestions: Update the paper command block (and any other shipped examples) to use pred solve ... --solver brute-force, or add an ILP-capable reduction path before documenting bare pred solve.

Expected vs Actual Outcome

  • pred list: expected yes, actual yes.
  • pred show RootedTreeArrangement: expected yes, actual yes.
  • pred create --example RootedTreeArrangement: expected yes, actual yes.
  • pred solve <example>: expected yes, actual no — failed with No reduction path from RootedTreeArrangement to ILP or ILP solver found no solution. Try \--solver brute-force`.`
  • pred solve <example> --solver brute-force: expected yes, actual yes.
  • pred evaluate <example> --config 0,0,1,2,0,1,2,3: expected yes, actual yes.

Issues Found

  • Confirmed — Important: The documented canonical solve path is broken. Reproduced with pred create --example RootedTreeArrangement followed by pred solve <file>; the model has no ILP reduction path.
  • Confirmed — Minor: pred list / pred show expose O(2^num_vertices), which does not match the only implemented brute-force encoding.

Suggestions

  • Add a CLI regression that exercises pred create --example RootedTreeArrangement plus solving the example.
  • Change the shipped pred solve examples for RootedTreeArrangement to --solver brute-force, unless an ILP reduction path is added.
  • Fix the declared complexity metadata to reflect the implemented search space, or implement and document a real 2^n-style exact algorithm.

Generated by review-pipeline

GiggleLiu and others added 2 commits March 22, 2026 19:53
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RootedTreeArrangement has no ILP reduction path, so the default
pred solve fails. Add --solver brute-force as required.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@GiggleLiu GiggleLiu mentioned this pull request Mar 22, 2026
3 tasks
@GiggleLiu GiggleLiu merged commit 94063e7 into main Mar 22, 2026
3 checks passed
@GiggleLiu GiggleLiu deleted the issue-407 branch April 12, 2026 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Model] RootedTreeArrangement

1 participant